-
Notifications
You must be signed in to change notification settings - Fork 604
Rollback to previous deployment if update fails [WIP] #1387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| if rollBackErr != nil { | ||
| return nil, "", err | ||
| } | ||
| return nil, "update failed; rollbacked to previous deployment", nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might still be good to return the error, in case the user can do anything about it
| return nil, "", err | ||
| rollBackErr := rollBack(prevVirtualService) | ||
| if rollBackErr != nil { | ||
| return nil, "", err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be good to return both errors, and say something along the lines of “update failed” with the error, and “During rollback, another error occurred” with that error
|
|
||
| if err := applyK8sResources(api, prevDeployment, prevService, prevVirtualService); err != nil { | ||
| go deleteK8sResources(api.Name) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we still delete what may have been created?
|
|
||
| return nil, "", err | ||
| } | ||
| err = operator.AddAPIToAPIGateway(*api.Networking.Endpoint, api.Networking.APIGateway, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove from api gateway if this fails, like we do for batch APIs?
|
|
||
| if prevVirtualService.Labels["specID"] != api.SpecID { | ||
| if err := config.AWS.UploadJSONToS3(api, config.Cluster.Bucket, api.Key); err != nil { | ||
| return nil, "", errors.Wrap(err, "upload api spec") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(above) Should we remove from API Gateway if adding it failed, like we do in batch?
| return err | ||
| } | ||
|
|
||
| if err := operator.UpdateAPIGatewayK8s(prevVirtualService, prevAPI, false); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the third arg be “true”?
closes #1250
checklist:
make testandmake lint